workaround for IE model + clipping planes shader bug #6584
Conversation
|
@likangning93, thanks for the pull request! Maintainers, we have a signed CLA from @likangning93, so you can review this at any time. I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
Source/Scene/Model.js
Outdated
| // Internet Explorer seems to have problems with discard after too many levels of indirection: | ||
| // https://github.com/AnalyticalGraphicsInc/cesium/issues/6575. | ||
| // In this case, log depth modifications are empty wrappers and can be omitted. | ||
| if (!FeatureDetection.isInternetExplorer()) { |
There was a problem hiding this comment.
Why not detect if log depth is supported instead of checking IE specifically?
There was a problem hiding this comment.
It's an IE bug more than a log depth bug, so I think this is more descriptive.
We could also check inside the modified functions for log depth support and just not modify the shader, but I'm not sure if there was some reason that wasn't happening already.
Maybe the current design was to keep log depth toggling to a single shader define for simplicity?
We'd also have to wire access to the context down to the modifier functions too.
There was a problem hiding this comment.
Did you try other browsers/OS's without log depth just to be sure? My main concern is that we don't know WHY it's crashing in IE, right? We just assume it's a bug in IE and not something non-standard in our own code.
There was a problem hiding this comment.
Clipping planes and picking with clipping planes seem to work fine on Chrome on my Android phone, which doesn't have the extensions needed for log depth.
There was a problem hiding this comment.
OK, cool thanks for checking.
| @@ -2041,10 +2049,13 @@ define([ | |||
| var drawVS = modifyShader(vs, id, model._vertexShaderLoaded); | |||
| var drawFS = modifyShader(finalFS, id, model._fragmentShaderLoaded); | |||
|
|
|||
| drawVS = ModelUtility.modifyVertexShaderForLogDepth(drawVS, toClipCoordinatesGLSL); | |||
| drawFS = ModelUtility.modifyFragmentShaderForLogDepth(drawFS); | |||
| if (!FeatureDetection.isInternetExplorer()) { | |||
Source/Scene/Model.js
Outdated
| @@ -2058,8 +2069,13 @@ define([ | |||
| pickFS = modifyShaderForClippingPlanes(pickFS, clippingPlaneCollection); | |||
| } | |||
|
|
|||
| pickVS = ModelUtility.modifyVertexShaderForLogDepth(pickVS, toClipCoordinatesGLSL); | |||
| pickFS = ModelUtility.modifyFragmentShaderForLogDepth(pickFS); | |||
| // Internet Explorer seems to have problems with discard after too many levels of indirection: | |||
There was a problem hiding this comment.
I would move this comment to the first check instead of last so readers encounter it sooner.
|
Thanks @likangning93. I don't have any comments to add, the bug is fixed. |
Fixes #6575